feat(datepicker): add year selection mode #8565
Conversation
googlebot
added
the
cla: yes
src/lib/datepicker/calendar.ts
Outdated
| this._dateAdapter.createDate(curYear - curYear % 24, 0, 1)); | ||
| let lastYear = this._dateAdapter.getYearName( | ||
| this._dateAdapter.createDate(curYear + yearsPerPage - 1 - curYear % 24, 0, 1)); | ||
| return `${firstYear} \u2013 ${lastYear}`; |
There was a problem hiding this comment.
Would this need to be internationalized?
There was a problem hiding this comment.
It probably should be. I don't think any of our *Intl objects currently have placeholders, so we'd have to make up some convention for that.
Should we switch to using the official Angular i18n stuff? It seems weird that we're just rolling our own.
src/lib/datepicker/calendar.ts
Outdated
| if (this._currentView == 'year') { | ||
| return this._dateAdapter.getYearName(this._activeDate); | ||
| } | ||
| let curYear = this._dateAdapter.getYear(this._activeDate); |
src/lib/datepicker/calendar.ts
Outdated
| let curYear = this._dateAdapter.getYear(this._activeDate); | ||
| let firstYear = this._dateAdapter.getYearName( | ||
| this._dateAdapter.createDate(curYear - curYear % 24, 0, 1)); | ||
| let lastYear = this._dateAdapter.getYearName( |
There was a problem hiding this comment.
How about selectedYear, firstYearInView and lastYearInView?
src/lib/datepicker/calendar.ts
Outdated
| @@ -208,29 +230,37 @@ export class MatCalendar<D> implements AfterContentInit, OnDestroy, OnChanges { | |||
| this._userSelection.emit(); | |||
| } | |||
|
|
|||
| /** Handles month selection in the multi-year view. */ | |||
| _yearSelected(year: D): void { | |||
There was a problem hiding this comment.
I combined this with _monthSelected and named the new method _goToDateInView
| } | ||
|
|
||
| /** Handles user clicks on the previous button. */ | ||
| _previousClicked(): void { | ||
| this._activeDate = this._monthView ? | ||
| this._activeDate = this._currentView == 'month' ? |
There was a problem hiding this comment.
Make a _isMonthView() function instead of repeating the comparison?
There was a problem hiding this comment.
Do we gain anything from the method? seems equally readable to what I'm doing here
src/lib/datepicker/calendar.ts
Outdated
| if (this._currentView == 'year') { | ||
| return this._dateAdapter.getYear(date1) == this._dateAdapter.getYear(date2); | ||
| } | ||
| return Math.floor(this._dateAdapter.getYear(date1) / yearsPerPage) == |
There was a problem hiding this comment.
Add comment like?
// Multi-year view| <tr><th class="mat-calendar-table-header-divider" colspan="4"></th></tr> | ||
| </thead> | ||
| <tbody mat-calendar-body | ||
| role="grid" |
There was a problem hiding this comment.
I think this also needs aria-readonly="true" since role="grid" treats it as editable by default
There was a problem hiding this comment.
ok, moved both of these to mat-calendar-body since they're the same for all of these views
|
@mmalerba this looks fine. but when You switch to Year view this isn't happening: Maybe same rule could be applied? Even better would be to hide years that aren't available to pick from (if I have min and max in same Year switching to Year view should be disabled) |
|
@Misiu yeah I should do that, I want to make a separate PR first that changes the way some of the logic works behind the scenes for that feature, then I'll add it here |
|
@mmalerba I've noticed that with latest changes I can't navigate to year list in demo in first comment (https://mmalerba-demo1.firebaseapp.com/datepicker) is this intended? |
|
oh I reused that demo for something else, there's currently no demo for this PR |
|
@mmalerba no worries :) I'll wait till it get merged. |
src/lib/datepicker/calendar.spec.ts
Outdated
| expect(calendarInstance._monthView).toBe(false); | ||
| expect(calendarInstance._currentView).toBe('multi-year'); | ||
|
|
||
| (<HTMLElement>calendarBodyEl.querySelector('.mat-calendar-body-active')).click(); |
There was a problem hiding this comment.
Use as HTMLElement (here and elsewhere)
jelbourn
added
pr: lgtm
target: minor
|
Good job guys! What is missing in this PR in order to be merged? |
|
@Misiu it seems that what you mentioned (if I have min and max in same Year switching to Year view should be disabled) is not implemented in current version. Do you have any idea about this? Sorry that can't find help with it.. |
|
@FergusZhou I'm not using Angular currently. Maybe @mmalerba can help You with this. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
fixes #5845